-
Notifications
You must be signed in to change notification settings - Fork 472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added the ability to see connectors that have been downloaded #1456
Conversation
@@ -30,6 +30,7 @@ export default class Storage { | |||
this.fs = require( 'fs' ); | |||
this.path = require( 'path' ); | |||
this.config = this.path.join( electron.remote.app.getPath( 'userData' ), 'hakuneko.' ); | |||
this.root = electron.remote.app.getPath( 'userData'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couldn't use this.config
since it contains hakuneko in the path. i don't like rewriting original paths or duplicating variable values, this needs to change
async registerfav(files) { | ||
try { | ||
for(let file of files) { | ||
try { | ||
let module = await import(file); | ||
let connector = new module.default(); | ||
if(this._favlist.find(c => c.id === connector.id)) { | ||
console.warn(`The connector "${connector.label}" with ID "${connector.id}" is already registered`); | ||
} else { | ||
this._favlist.push(connector); | ||
} | ||
} catch(error) { | ||
console.warn(`Failed to load connector "${file}"`, error); | ||
} | ||
} | ||
this._favlist.sort( ( a, b ) => { | ||
return ( a.label.toLowerCase() < b.label.toLowerCase() ? -1 : 1 ); | ||
} ); | ||
} catch(error) { | ||
console.warn(`Failed to load connector`, error); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only part I'm not happy with, since original register(files)
uses this._list
(which can't be rewritten) I've had to create a new register function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method is not needed anymore if you accept the suggested change based on Storage.mjs
(07/04 13:42) TODO:
|
@@ -212,6 +212,34 @@ | |||
</template> | |||
</div> | |||
</div> | |||
<div class="connectors"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this duplicated code should not be added.
Just introduce a new checkbox or something to toggle between the full list of websites and the websites that have been downloaded. In the code behind, assign the corresponding list to the property which is bound to the dom-repeat template in the UI.
onClick(evt) {
this.connectorList = evt.value ? /* downloaded website list */ this.favoriteList : /* full website list */;
}
@@ -212,6 +212,34 @@ | |||
</template> | |||
</div> | |||
</div> | |||
<div class="connectors"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as for the dark theme
@@ -911,4 +912,18 @@ export default class Storage { | |||
} ); | |||
} ); | |||
} | |||
|
|||
get downloadedConnectors() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just return a list of connector IDs (string), makes further processing much simpler.
} | ||
|
||
get list() { | ||
return this._list; | ||
} | ||
|
||
get favlist() { | ||
return this._favlist; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just get the connector IDs of downloaded lists as suggested in Storage.mjs
and then filter the full list based of the downloaded...
e.g.
let downloaded = Storage.getDownloadedConnectorIDs();
return this._list.filter(connector => downloaded.includes(connector.id));
@@ -24,16 +25,22 @@ export default class Connectors { | |||
]; | |||
let userPlugins = await this._loadPlugins('hakuneko://plugins/'); | |||
let internalPlugins = await this._loadPlugins('hakuneko://cache/mjs/connectors/'); | |||
let favoritePlugins = Engine.Storage.downloadedConnectors; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed anymore if you accept the suggested change based on Storage.mjs
|
||
await this.register(systemPlugins); | ||
await this.register(userPlugins); | ||
await this.register(internalPlugins); | ||
await this.registerfav(favoritePlugins); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed anymore if you accept the suggested change based on Storage.mjs
@@ -3,6 +3,7 @@ export default class Connectors { | |||
constructor(ipc) { | |||
ipc.listen('on-connector-protocol-handler', this._onConnectorProtocolHandler.bind(this)); | |||
this._list = []; | |||
this._favlist = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed anymore if you accept the suggested change based on Storage.mjs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but i would strongly recommend to take a look at the suggestions especially in the UI and Storage as these will greatly simplify the implementation and reuse existing concepts.
again thanks for your feedback, going to pick this up asap |
Is this PR still active? |
Will close for now, maybe re-opened when work is continued |
With this addition you're able to see what connectors have already been downloaded,
since there's such great amount of connectors it can be daunting to know which one you used before. this feature is based on the reqeust from #535 and #1114
this code is far from optimal, best would be to convert
Favorite.mjs
intoConnectors.mjs
but to proof it works i've done it this way. made it easier for me to play around without rewriting original code.
(07/04 13:00) Edit based on feedback from Ronny:
Storage.mjs
and use already existing methods(07/04 13:42) TODO:
this.root
variable to local variable usingelectron.remote.app.getPath('userData')
register(file)
andregisterfav(file)
to reduce duplicate code